From 072d89ecb48e219d7249ec7d51118947cf2d33a5 Mon Sep 17 00:00:00 2001
From: Joonas Koivunen <joonas.koivunen@gmail.com>
Date: Fri, 14 Apr 2017 08:38:36 +0300
Subject: [PATCH] git: refactor update_submodule out of loop

---
 src/cargo/sources/git/utils.rs | 92 ++++++++++++++++++----------------
 1 file changed, 50 insertions(+), 42 deletions(-)

diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs
index 78f50061f..fdf729b5b 100644
--- a/src/cargo/sources/git/utils.rs
+++ b/src/cargo/sources/git/utils.rs
@@ -296,52 +296,60 @@ impl<'a> GitCheckout<'a> {
             info!("update submodules for: {:?}", repo.workdir().unwrap());
 
             for mut child in repo.submodules()?.into_iter() {
-                child.init(false)?;
-                let url = child.url().chain_error(|| {
-                    internal("non-utf8 url for submodule")
-                })?;
+                update_submodule(repo, &mut child, cargo_config).chain_error(||
+                    internal(
+                        format!("failed to update submodule `{}`",
+                            child.name().unwrap_or("")))
+                    )?;
+            }
+            Ok(())
+        }
 
-                // A submodule which is listed in .gitmodules but not actually
-                // checked out will not have a head id, so we should ignore it.
-                let head = match child.head_id() {
-                    Some(head) => head,
-                    None => continue,
-                };
-
-                // If the submodule hasn't been checked out yet, we need to
-                // clone it. If it has been checked out and the head is the same
-                // as the submodule's head, then we can bail out and go to the
-                // next submodule.
-                let head_and_repo = child.open().and_then(|repo| {
-                    let target = repo.head()?.target();
-                    Ok((target, repo))
-                });
-                let repo = match head_and_repo {
-                    Ok((head, repo)) => {
-                        if child.head_id() == head {
-                            continue
-                        }
-                        repo
-                    }
-                    Err(..) => {
-                        let path = repo.workdir().unwrap().join(child.path());
-                        let _ = fs::remove_dir_all(&path);
-                        git2::Repository::clone(url, &path)?
+        fn update_submodule(parent: &git2::Repository, child: &mut git2::Submodule, cargo_config: &Config) -> CargoResult<()> {
+            child.init(false)?;
+            let url = child.url().chain_error(|| {
+                internal("non-utf8 url for submodule")
+            })?;
+
+            // A submodule which is listed in .gitmodules but not actually
+            // checked out will not have a head id, so we should ignore it.
+            let head = match child.head_id() {
+                Some(head) => head,
+                None => return Ok(()),
+            };
+
+            // If the submodule hasn't been checked out yet, we need to
+            // clone it. If it has been checked out and the head is the same
+            // as the submodule's head, then we can bail out and go to the
+            // next submodule.
+            let head_and_repo = child.open().and_then(|repo| {
+                let target = repo.head()?.target();
+                Ok((target, repo))
+            });
+            let repo = match head_and_repo {
+                Ok((head, repo)) => {
+                    if child.head_id() == head {
+                        return Ok(())
                     }
-                };
+                    repo
+                }
+                Err(..) => {
+                    let path = parent.workdir().unwrap().join(child.path());
+                    let _ = fs::remove_dir_all(&path);
+                    git2::Repository::clone(url, &path)?
+                }
+            };
 
-                // Fetch data from origin and reset to the head commit
-                let refspec = "refs/heads/*:refs/heads/*";
-                fetch(&repo, url, refspec, cargo_config).chain_error(|| {
-                    internal(format!("failed to fetch submodule `{}` from {}",
-                                     child.name().unwrap_or(""), url))
-                })?;
+            // Fetch data from origin and reset to the head commit
+            let refspec = "refs/heads/*:refs/heads/*";
+            fetch(&repo, url, refspec, cargo_config).chain_error(|| {
+                internal(format!("failed to fetch submodule `{}` from {}",
+                                 child.name().unwrap_or(""), url))
+            })?;
 
-                let obj = repo.find_object(head, None)?;
-                repo.reset(&obj, git2::ResetType::Hard, None)?;
-                update_submodules(&repo, cargo_config)?;
-            }
-            Ok(())
+            let obj = repo.find_object(head, None)?;
+            repo.reset(&obj, git2::ResetType::Hard, None)?;
+            update_submodules(&repo, cargo_config)
         }
     }
 }
-- 
2.30.2